-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CSM] Fix core vital legend background #78273
[CSM] Fix core vital legend background #78273
Conversation
Pinging @elastic/uptime (Team:uptime) |
Pinging @elastic/apm-ui (Team:apm) |
@elasticmachine merge upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested in dark mode and UI LGTM.
Left a bunch of comments but they're mostly optional or wellness checks.
x-pack/plugins/apm/public/components/app/RumDashboard/CoreVitals/translations.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/apm/public/components/app/RumDashboard/CoreVitals/PaletteLegends.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/apm/public/components/app/RumDashboard/CoreVitals/PaletteLegends.tsx
Show resolved
Hide resolved
@@ -28,7 +29,7 @@ export function CoreVitals({ data, loading }: Props) { | |||
<EuiFlexItem> | |||
<CoreVitalItem | |||
title={LCP_LABEL} | |||
value={lcp ? lcp + ' s' : '0'} | |||
value={formatToSec(lcp, 'ms')} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the output of this function going to be a string presented to the user, or is 'ms' an enumerable string?
If this will cause ms
to show to the user make sure the output is translated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is already done in another PR
x-pack/plugins/apm/public/components/app/RumDashboard/UXMetrics/index.tsx
Show resolved
Hide resolved
lcp: ((lcp?.values['50.0'] || 0) / 1000).toFixed(2), | ||
tbt: tbt?.values['50.0'] || 0, | ||
fcp: fcp?.values['50.0'] || 0, | ||
fid: fid?.values['50.0'] ?? 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this value have > 2 decimal digits? Ignore if the answer is no, or formatting it as such is no longer important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's done in frontend
💚 Build SucceededMetrics [docs]async chunks size
History
To update your PR or re-run it, just comment with: |
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
…-to-timeline * 'master' of github.com:elastic/kibana: (22 commits) update apm index pattern (elastic#78732) 78024: move transform out of dataset (elastic#78216) [QA][Code Coverage] Upload the coverage static site before ingestion (elastic#78695) [Discover] Make _source field not clickable (elastic#78698) [Fleet] Rename Ingest Manager => Fleet, Fleet => Agents in the UI (elastic#78685) [APM] Review feedback from distribution + transaction metrics (elastic#78752) [Ingest pipelines] Add ability to stop pipeline simulation (elastic#78183) [CSM] Fix core vital legend background (elastic#78273) [Usage Collection] [schema] Support spreads + `canvas` definition (elastic#78481) fix lodash imports (elastic#78456) [Maps] Add layer type preview icons (elastic#78650) [APM] Use transaction metrics for distribution charts (elastic#78484) [Uptime] Ml anomaly alert edit (elastic#76909) [ML] Limit exposing shared static code through ml/public/index.ts. (elastic#77745) making expression debug info serializable (elastic#78727) fix lodahs imports in app-arch code (elastic#78582) Make Field a React.lazy export (elastic#78483) [Security Solution] Improves detections tests (elastic#77295) [TSVB] Different field format on different series is ignored (elastic#78138) RFC: Improve saved object migrations (elastic#66056) ...
Summary
Fixes: #77283
Use dark theme var when dark mode is selected for background color.
Added text alongside core vital average, added a popover help icon